Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build with arm versions #528

Merged
merged 3 commits into from
Aug 23, 2016
Merged

Build with arm versions #528

merged 3 commits into from
Aug 23, 2016

Conversation

franciscocpg
Copy link
Member

For this to work with gox this gox PR needs to be merged.
While it does not happen (And I don't know if someday it will), it's possible to use the fork https://github.com/franciscocpg/gox.
After merging this PR, it is necessary to run make dist and publish the new arm dists to the latest glide release.
Also this glide.sh PR needs to be merged so one can install glide on a arm device with curl https://glide.sh/get | sh.

@@ -23,8 +23,10 @@ bootstrap-dist:
build-all:
gox -verbose \
-ldflags "-X main.version=${VERSION}" \
-ldflags "-X main.version=${VERSION}" \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this line is duplicated?

@franciscocpg
Copy link
Member Author

franciscocpg commented Jul 28, 2016

Hi @dnephin. Thanks for the warning.
Fixed!

@@ -24,7 +24,8 @@ build-all:
gox -verbose \
-ldflags "-X main.version=${VERSION}" \
-os="linux darwin windows " \
-arch="amd64 386" \
-arch="amd64 386 armv5 armv6 armv7 arm64" \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried it out and I don't think armv{5,6,7} are valid. It just ignores the arch's it doesn't know about.

I believe only arm and arm64 are valid.

Copy link
Member Author

@franciscocpg franciscocpg Jul 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As explained in the first conversation of this PR you must use a gox fork to work.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw, I built with just arm and it's working fine on armv7.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, but as stated here

In cross compilation situations, it is recommended that you always set an appropriate GOARM value along with GOARCH.

@mattfarina
Copy link
Member

I'm keeping an eye on it and we'll have this in some manner before the next release.

I want to see if Mitchell will merge upstream.

@mattfarina mattfarina added this to the 0.12 milestone Aug 19, 2016
@mattfarina
Copy link
Member

It seems Mitchell has not merged this upstream and has not responded to emails either. I will likely look at using the fork for now.

@franciscocpg
Copy link
Member Author

@mattfarina
Do you think that it makes sense to have a Masterminds/gox fork?

@mattfarina mattfarina merged commit 9f0d3a9 into Masterminds:master Aug 23, 2016
mattfarina added a commit that referenced this pull request Aug 23, 2016
This includes Merge pull request #528 from franciscocpg/master to build arm
@mattfarina
Copy link
Member

@franciscocpg thanks for the contribution.

For gox I'm pulling from your repo for the time being. If in the next couple release cycles upstream gox doesn't merge the work we'll deal with it.

@mattfarina
Copy link
Member

@franciscocpg I'm going to start a Masterminds/gox fork... as you have suggested. I'm going to add you to the repo. This was a collective we can maintain it.

Note, I've emailed Mitchell about gox and if anything comes in I'll let you know.

@franciscocpg
Copy link
Member Author

Nice @mattfarina! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants